-
Notifications
You must be signed in to change notification settings - Fork 587
Support custom CRDs for ExtensionServer in Standalone Mode #7331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support custom CRDs for ExtensionServer in Standalone Mode #7331
Conversation
1f7a84b to
d66478e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7331 +/- ##
==========================================
+ Coverage 72.26% 72.41% +0.14%
==========================================
Files 231 231
Lines 34071 34128 +57
==========================================
+ Hits 24622 24714 +92
+ Misses 7674 7646 -28
+ Partials 1775 1768 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@shawnh2 i have added a test scenario which shows we have the same problem for the other 2 resources fields in the extension manager in standalone mode.... i think i will address this in this PR as well.. (or would you prefer a separate Issue/PR ?) |
shawnh2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks
internal/gatewayapi/resource/load.go
Outdated
| Spec: typedSpec.(gwapiv1b1.ReferenceGrantSpec), | ||
| } | ||
| resources.ReferenceGrants = append(resources.ReferenceGrants, referenceGrant) | ||
| default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we improve this logic and avoid labels
one recommendation is to setup a gvkMap in the beginning of the func, populate it based off EnvoyGateway and use it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this is a good idea!
| extGKs = append(extGKs, schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}) | ||
| } | ||
| // Include policy resources so extension-defined custom policies are recognised. | ||
| for _, gvk := range r.EnvoyGateway.ExtensionManager.PolicyResources { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a bug fix @guydc
|
i think i found yet another problem in standalone mode :D is this intentional or just got forgotten? |
|
the test shows the problem ... when I register the tested index in the offline_controller, it succeeds... |
| un.SetNamespace(namespace) | ||
| switch category { | ||
| case extFilter, extBackend: | ||
| resources.ExtensionRefFilters = append(resources.ExtensionRefFilters, *un) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this logic right of adding the ext backend into ext ref flters ?
cc @Xunzhuo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think so, the controller does it the same way
#7331 (comment)
|
strange.. the failing conformance test succeeds locally on my machine... |
6b145ed to
19fe896
Compare
3825255 to
7c44f70
Compare
…nt scheme Teach the offline controller to recognise custom extension-server policy GVKs supplied by an extension so they can be decoded and later processed by the translator. - newOfflineGatewayAPIClient now accepts extServerPoliciesGVKs - each GVK is registered as Unstructured/UnstructuredList in the fake client's scheme - call-site updated to pass the collected policy GVKs Signed-off-by: daum3ns <[email protected]>
…backend resource registration Signed-off-by: daum3ns <[email protected]>
when reading from yaml Signed-off-by: daum3ns <[email protected]>
Add policy resources from ExtensionManager to extGKs so custom extension policies are recognised during translation. Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
server and policy resource Signed-off-by: daum3ns <[email protected]>
…f an EnvoyGateway configuration containing a standalone extension server. Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
yaml and it gets the default namespace Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
can check custom resource definitions Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
ExtensionManager.resources and ExtensionManager.backendResources, use assertions Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
… ExtensionRefFilters Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
extensionmanager.backendResources Signed-off-by: daum3ns <[email protected]>
instead of srv.EnvoyGateway (replace function parameter, adapt callers and tests) Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
and to avoid the loop over every resource in extensionanager. Signed-off-by: daum3ns <[email protected]>
unregistered list index Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>
registered indices Signed-off-by: daum3ns <[email protected]>
7c44f70 to
b426305
Compare
This reverts commit 2e48e33. Signed-off-by: daum3ns <[email protected]>
Signed-off-by: daum3ns <[email protected]>

This change enables the file-based (standalone) provider to understand and load custom resources that are defined by an extension server.
LoadResourcesFromYAMLBytesnow accepts a server config pointer. When a resource kind is unknown, the loader checks the configured ExtensionManager and, if the GVK matches a declared policy resource, stores the object as an ExtensionServerPolicynewOfflineGatewayAPIClientfunction now accepts extensionServerPoliciesGVKs, which are then registered in the scheme. this allows the offline reconciler to manipulate custom resourcesFixes #7141
Release Notes: Yes/No
Notes:
I found out that we most probably have the same problem with ExtensionManagers
backendResourcesandresourcesfield (at least the config_loader test can easily be extended to show that the registration in the scheme is missing).I think it could be fixed in a similar way (although not sure whether the translator needs to be adapted as well). See the commented code snippets...
However, I'm very new to envoy-gateway so I'm still raising this PR to get a first, much appreciated feedback whether it makes sense to continue in this direction..